Skip to content

test(install-proxy): run iterations sequentially to avoid squid saturation - #34175

Closed
robobun wants to merge 1 commit into
mainfrom
farm/ac260066/deflake-install-proxy
Closed

robobun wants to merge 1 commit into
mainfrom
farm/ac260066/deflake-install-proxy

Conversation

@robobun

@robobun robobun commented Jul 14, 2026 •

Copy link
Copy Markdown
Collaborator

What

test/cli/install/bun-install-proxy.test.ts has been going red on first attempt in roughly 30% of PR builds since 2026-07-11 (and briefly on 2026-07-06), always with the same shape:

error: failed to install with exit code 143
      at bun-install-proxy.test.ts:87
✗ bun install with proxy with big packages [20020.05ms]

Four of the five concurrent bun install processes finish in ~1s; one stalls at "Resolving dependencies" until the 20s Bun.spawn timeout SIGTERMs it (143 = 128 + SIGTERM). Seen in build 72962 and many others (72954, 72935, 72927, 72916, 72912, 72910, 72906, 72903, 72900, ...).

Not a code regression

I checked merge bases for every failed build on 2026-07-11: builds based on the same main commit (55ba6e453c, ab84aa22d2, 9657f37896) are both clean and red within the same hour. The flake correlates with wall-clock time, not with what was merged. Before 2026-07-06 the rate was 0/60 across several sampled days.

Cause

The test spawns five bun install processes in parallel, each with the default --network-concurrency of 48. All traffic goes through a single squid container to registry.npmjs.org, so up to ~240 CONNECT tunnels compete for the one proxy. Under current npm/network conditions one of the five occasionally gets starved past 20s.

Fix

Run the five iterations sequentially instead of concurrently, drop the 20s per-spawn timeout, and rely on the 120s test-level timeout for hang detection. A genuine proxy hang (what #19771 fixed and this test guards) still fails the test; transient npm slowness no longer does. Also pipe stdout/stderr and print them on non-zero exit so a real failure is diagnosable.

The test still hits the live npm registry; making it hermetic is a larger change that #33115 is already moving toward.


[stamp-90s] gate passed · iteration 0 · 1 files touched

passes on PR (with fix)
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/cli/install/bun-install-proxy.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/cli/install/bun-install-proxy.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (158e3f19f)

test/cli/install/bun-install-proxy.test.ts:
failed to connect to the docker API at unix:///var/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /var/run/docker.sock: connect: no such file or directory

 0 pass
 0 fail
Ran 0 tests across 1 file. [2.31s]
Exit: 0
diff hotspot
test/cli/install/bun-install-proxy.test.ts | 57 +++++++++++++++---------------
 1 file changed, 28 insertions(+), 29 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                        reads  edits  tests
test/cli/install/bun-install-proxy.test.ts      1      1      0

…ation

The test spawns 5 concurrent bun install processes, each with a default
network concurrency of 48, pushing up to ~240 simultaneous CONNECT
tunnels through a single squid proxy to registry.npmjs.org. One of the
five occasionally stalls past the 20s per-spawn timeout and gets
SIGTERM'd (exit 143), showing up as a ~30% first-attempt failure rate
across PR builds since 2026-07-11.

The flake is not a code regression: identical merge bases produce both
clean and red runs on the same day, so the trigger is environmental
(squid or npm), not bun.

Run the five iterations sequentially so each install has the proxy to
itself, drop the per-spawn timeout, and rely on the 120s test timeout
to catch a genuine proxy hang (the #19771 regression this test exists
for). Pipe stdout/stderr so a real failure is diagnosable.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e5a529cc-486f-47bc-b795-46dde702a538

📥 Commits

Reviewing files that changed from the base of the PR and between cc0c1e8 and 158e3f1.

📒 Files selected for processing (1)
  • test/cli/install/bun-install-proxy.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Jul 14, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:49 PM PT - Jul 14th, 2026

❌ @robobun, your commit 158e3f1 has 2 failures in Build #72994 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 34175

That installs a local version of the PR into your bun-34175 executable, so you can run:

bun-34175 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — test-only de-flake, no production code touched.

What was reviewed:

  • Sequential loop + 120s test-level timeout still catches the #19771 proxy-hang regression; parallelism in the original was redundancy, not part of the repro.
  • Subprocess handling matches repo conventions: await using proc, concurrent Promise.all drain of stdout/stderr/exited, output logged before expect(exitCode).
  • try/finally ensures temp dirs are cleaned up on failure; expect import added.
Extended reasoning...

Overview

Single-file change to test/cli/install/bun-install-proxy.test.ts. Converts five parallel bun install spawns into a sequential loop, drops the 20s per-spawn timeout (which was producing exit 143 / SIGTERM under transient npm slowness), raises the test-level timeout from 60s to 120s, switches to await using proc with piped stdout/stderr drained concurrently, and wraps each iteration in try/finally so the temp dir is removed even on assertion failure. Adds the missing expect import.

Security risks

None. Test-only change; no runtime, install, or proxy code paths are modified. Environment variables and spawn arguments are unchanged.

Level of scrutiny

Low. This is a CI flake fix confined to a single test file. The PR description gives a clear mechanical explanation (5 × 48 concurrent CONNECT tunnels through one squid instance → occasional starvation past 20s) and demonstrates the failure correlates with wall-clock time rather than any merged commit. The key question — does serializing the iterations weaken regression coverage for #19771? — is addressed: the original test ran 5 iterations for redundancy ("run multiple times to make sure it's not flaky"), not because concurrency was required to reproduce the hang. A genuine proxy hang will still trip the 120s test-level timeout.

Other factors

The rewrite aligns with the repo's documented test patterns in CLAUDE.md: await using for subprocess cleanup, Promise.all([stdout.text(), stderr.text(), exited]) to avoid pipe-buffer deadlock, and printing stdout/stderr before asserting the exit code so failures are diagnosable. Cleanup moved into try/finally is strictly safer than the previous .finally() chain. No existing assertions were weakened. No outstanding reviewer comments.

@robobun

robobun commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

CI on build 72994: bun-install-proxy.test.ts passed on every lane, no retries needed.

Remaining red is unrelated to this test-only diff:

  • test-worker-message-port-transfer-terminate.js and test-net-connect-memleak.js are both marked [pre-existing] (also red on main).
  • verify-baseline timeouts (segfault in simd-baseline.test.ts) are hitting many concurrent PR builds right now (72986, 72987, 72989, 72990, 72991, 72993).

Ready for review.

@robobun

robobun commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator Author

Heads-up: #39243 changes the top of test/cli/install/bun-install-proxy.test.ts (squid now comes from the compose service via ensure(), and SQUID_URL is assigned in beforeAll); the install loop this PR rewrites still reads SQUID_URL, so the rebase should be small.

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

This is Claude (an AI assistant) writing on Jarred's behalf.

Included in #44968.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants